Conversation
…ion (#5613) AsyncDelegateAssertion implements both IAssertionSource<object?> and IAssertionSource<Task>, breaking C# generic inference for IsNotNull, IsNull, IsSameReferenceAs, and IsNotSameReferenceAs on Assert.That(Task). - IsNotNull / IsNull produced a misleading CS1929 pointing at JsonElement - IsSameReferenceAs / IsNotSameReferenceAs produced CS0411 Add instance methods that forward to the IAssertionSource<Task> interface, matching the existing pattern used for IsCompleted / IsFaulted / IsCanceled.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This is a well-scoped, targeted fix for the C# inference ambiguity caused by AsyncDelegateAssertion implementing both IAssertionSource<object?> and IAssertionSource<Task>.
What's Good
- Correct pattern: The four new methods exactly mirror the existing forwarding pattern used for
IsCompleted,IsFaulted,IsCanceled,IsTypeOf, etc. — no new approaches, no surprises. - Proper regression tests: All four methods get dedicated tests in
Issue5613Tests.csthat confirm they compile and pass, which is the exact invariant being protected. - Public API snapshots updated: All four TFMs covered. Good hygiene.
- Minimal scope: No unrelated refactoring or cleanup included.
Architectural Observation (not a blocker)
The class now has roughly 20+ instance-method forwarders to resolve the dual-interface ambiguity, and this PR adds 4 more. The concern is that this is an open-ended maintenance tax: every future generic assertion added to IAssertionSource<T> (e.g. a hypothetical IsEquivalentTo, HasSameHashCodeAs, etc.) may silently re-introduce the same CS0411/CS1929 problem on AsyncDelegateAssertion unless a forwarder is also added.
A more sustainable approach to consider for a follow-up (not this PR) would be to expose the IAssertionSource<Task> view as a distinct fluent entry point — e.g. Assert.That(task).AsTask() returning IAssertionSource<Task> — so the dual-interface is only needed internally, not as a public API surface. That eliminates the ambiguity entirely without requiring per-method forwarders. Given findings #2, #4, #5 are still outstanding, it might be worth tracking this as part of the broader #5613 clean-up.
Minor Note
The XML doc on IsNotNull() says "Without this instance method, Assert.That(task).IsNotNull() would fail generic inference..." — this is exactly the right kind of WHY comment and is appreciated here.
Verdict: Approve. The fix is correct, minimal, well-tested, and consistent with the existing class patterns.
This was referenced Apr 18, 2026
intellitect-bot
pushed a commit
to IntelliTect/EssentialCSharp.Web
that referenced
this pull request
Apr 21, 2026
Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.35.2 to 1.37.10. <details> <summary>Release notes</summary> _Sourced from [TUnit's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.37.10 <!-- Release notes generated using configuration in .github/release.yml at v1.37.10 --> ## What's Changed ### Other Changes * docs(test-filters): add migration callout for --filter → --treenode-filter by @johnkattenhorn in thomhurst/TUnit#5628 * fix: re-enable RPC tests and modernize harness (#5540) by @thomhurst in thomhurst/TUnit#5632 * fix(mocks): propagate [Obsolete] and null-forgiving raise dispatch (#5626) by @JohnVerheij in thomhurst/TUnit#5631 * ci: use setup-dotnet built-in NuGet cache by @thomhurst in thomhurst/TUnit#5635 * feat(playwright): propagate W3C trace context into browser contexts by @thomhurst in thomhurst/TUnit#5636 ### Dependencies * chore(deps): update tunit to 1.37.0 by @thomhurst in thomhurst/TUnit#5625 ## New Contributors * @johnkattenhorn made their first contribution in thomhurst/TUnit#5628 * @JohnVerheij made their first contribution in thomhurst/TUnit#5631 **Full Changelog**: thomhurst/TUnit@v1.37.0...v1.37.10 ## 1.37.0 <!-- Release notes generated using configuration in .github/release.yml at v1.37.0 --> ## What's Changed ### Other Changes * fix: stabilize flaky tests across analyzer, OTel, and engine suites by @thomhurst in thomhurst/TUnit#5609 * perf: engine hot-path allocation wins (#5528 B) by @thomhurst in thomhurst/TUnit#5610 * feat(analyzers): detect collection IsEqualTo reference equality (TUnitAssertions0016) by @thomhurst in thomhurst/TUnit#5615 * perf: consolidate test dedup + hook register guards (#5528 A) by @thomhurst in thomhurst/TUnit#5612 * perf: engine discovery/init path cleanup (#5528 C) by @thomhurst in thomhurst/TUnit#5611 * fix(assertions): render collection contents in IsEqualTo failure messages (#5613 B) by @thomhurst in thomhurst/TUnit#5619 * feat(analyzers): code-fix for TUnit0015 to insert CancellationToken (#5613 D) by @thomhurst in thomhurst/TUnit#5621 * fix(assertions): add Task reference forwarders on AsyncDelegateAssertion by @thomhurst in thomhurst/TUnit#5618 * test(asp-net): fix race in FactoryMethodOrderTests by @thomhurst in thomhurst/TUnit#5623 * feat(analyzers): code-fix for TUnit0049 to insert [MatrixDataSource] (#5613 C) by @thomhurst in thomhurst/TUnit#5620 * fix(pipeline): isolate AOT publish outputs to stop clobbering pack DLLs (#5622) by @thomhurst in thomhurst/TUnit#5624 ### Dependencies * chore(deps): update tunit to 1.36.0 by @thomhurst in thomhurst/TUnit#5608 * chore(deps): update modularpipelines to 3.2.8 by @thomhurst in thomhurst/TUnit#5614 **Full Changelog**: thomhurst/TUnit@v1.36.0...v1.37.0 ## 1.36.0 <!-- Release notes generated using configuration in .github/release.yml at v1.36.0 --> ## What's Changed ### Other Changes * fix: don't render test's own trace as Linked Trace in HTML report by @thomhurst in thomhurst/TUnit#5580 * fix(docs): benchmark index links 404 by @thomhurst in thomhurst/TUnit#5587 * docs: replace repeated benchmark link suffix with per-test descriptions by @thomhurst in thomhurst/TUnit#5588 * docs: clearer distributed tracing setup and troubleshooting by @thomhurst in thomhurst/TUnit#5597 * fix: auto-suppress ExecutionContext flow for hosted services (#5589) by @thomhurst in thomhurst/TUnit#5598 * feat: auto-align DistributedContextPropagator to W3C by @thomhurst in thomhurst/TUnit#5599 * feat: TUnit0064 analyzer + code fix for direct WebApplicationFactory inheritance by @thomhurst in thomhurst/TUnit#5601 * feat: auto-propagate test trace context through IHttpClientFactory by @thomhurst in thomhurst/TUnit#5603 * feat: TUnit.OpenTelemetry zero-config tracing package by @thomhurst in thomhurst/TUnit#5602 * fix: restore [Obsolete] members removed in v1.27 (#5539) by @thomhurst in thomhurst/TUnit#5605 * feat: generalize OTLP receiver for use outside TUnit.Aspire by @thomhurst in thomhurst/TUnit#5606 * feat: auto-configure OpenTelemetry in TestWebApplicationFactory SUT by @thomhurst in thomhurst/TUnit#5607 ### Dependencies * chore(deps): update tunit to 1.35.2 by @thomhurst in thomhurst/TUnit#5581 * chore(deps): update dependency typescript to ~6.0.3 by @thomhurst in thomhurst/TUnit#5582 * chore(deps): update dependency coverlet.collector to v10 by @thomhurst in thomhurst/TUnit#5600 **Full Changelog**: thomhurst/TUnit@v1.35.2...v1.36.0 Commits viewable in [compare view](thomhurst/TUnit@v1.35.2...v1.37.10). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This was referenced Apr 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses findings #1 and #3 of issue #5613.
Assert.That(Task task)returnsAsyncDelegateAssertion, which implements bothIAssertionSource<object?>andIAssertionSource<Task>. The ambiguity breaks C# generic inference on a handful of otherwise-natural assertions:Assert.That(task).IsNotNull()/IsNull()→CS1929with a misleading diagnostic pointing atJsonElementAssertionExtensions.IsNotNull(IAssertionSource<JsonElement>)(compiler's fallback "best candidate").Assert.That(task).IsSameReferenceAs(other)/IsNotSameReferenceAs(other)→CS0411("type arguments cannot be inferred").Fix: add four instance methods on
AsyncDelegateAssertionthat forward to theIAssertionSource<Task>interface. This matches the pattern already used on the same class forIsCompleted/IsFaulted/IsCanceled/IsTypeOf/ etc., which exist for the same reason.No behavior change for existing callers — the forwarders resolve an inference ambiguity that previously required
((object)task)or<Task>annotations.Remaining findings (#2, #4, #5) will be addressed in follow-up PRs.
Test plan
TUnit.Assertions.Tests/Bugs/Issue5613Tests.cs(4 tests, one per method). Confirmed failing-to-compile before the fix, passing after.TUnit.Assertions.Testssuite passes on net10 (1978), net9 (1978), net8 (1926), net472 (1778).TUnit.PublicAPIsnapshots updated for all four TFMs (only change: the 4 new public methods onAsyncDelegateAssertion).